Leak checker #8687
Leak checker #8687
Conversation
…ailable FinalizationGroup is part of the WeakRef proposal for JavaScript, which is currently in "stage 2" of the standardization process: https://github.com/tc39/proposal-weakrefs This patch modifies Embind to automatically attach JS finalizers that will drop references to C++ objects held by JavaScript objects when those objects go away, and invoke destructors if a C++ object is no longer held by JS at all. Currently the only browser that implements the WeakRef proposal is Chromium in Git. To run it with weak refs, run as: ./chrome --js-flags=--harmony-weak-refs (runDestructor): Modify to take $$ pointer as arg, instead of wrapper handle. This is needed because we register a finalizer on the wrapper, but when the finalizer runs we don't have the wrapper any more: just the $$ pointer. (releaseClassHandle): Factor out this piece from ClassHandle_delete, implementing the guts of what's needed by a finalizer. (finalizationGroup, removeFinalizer, attachFinalizer): Add implementation of finalizers. (ClassHandle.delete): Remove finalizer when invoked explicitly. (makeClassHandle, ClassHandle.clone, ClassHandle.__construct): Register finalizers on wrappers, using the $$ pointer as the handle given the the finalizer and the key to unregister the object with the FinalizationGroup.
* src/embind/embind.js: If the user passes -s EMBIND_LEAKCHECK=1, warn instead of silently calling an object's destructor if it becomes collectable but wasn't explicitly destroy()'d by the user. * src/settings.js: Add EMBIND_LEAKCHECK=1 variable.
|
What is the benefit of finding leaks, instead of enabling the automatic cleanup? |
|
@kripken The use case is where you are writing both the C/C++ and JS sides of an app and you want to manually manage memory on the JS side. But, it's hard to be sure that you've called For me it's not necessarily leak-check versus automatic cleanup -- to my mind when you enable leak-checking it's a developer tool, not production, so whether to to actually invoke the finalizer or not doesn't matter a whole lot. I think it could make sense to run the finalizer in this case, no problem. As another data point for this use case: https://www.hboehm.info/gc/leak.html Not quite the same but very similar. |
|
I see, thanks @wingo - that does sound useful! |
|
The deletion of the Re-opening and re-targeting to master. |
|
I'm going to stamp this. I love the idea of the feature and I trust that if it passes tests it's probably fine. I did not audit any possible performance implications. I would suggest a mechanism to detect leaks other than console.warn! It would be awesome to attach a callback that your application could use to report memory leaks, even to the application's telemetry. |
|
No strong opinion here, seems fine. Can merge after fixing CI and conflicts. |
|
This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant. |
This PR builds on #8474, adding support for leak-checking instead of automatic finalization.